fix(core): make getConnectedDataURL opts optional - #21736
fix(core): make getConnectedDataURL opts optional#21736SEPURI-SAI-KRISHNA wants to merge 1 commit into
Conversation
|
Thanks for your contribution! Please DO NOT commit the files in dist, i18n, and ssr/client/dist folders in a non-release pull request. These folders are for release use only. |
There was a problem hiding this comment.
Pull request overview
Fixes getConnectedDataURL to support its documented optional argument.
Changes:
- Normalizes omitted options to an empty object.
- Adds regression tests for omitted,
undefined, and explicit options.
Reviewed changes
Copilot reviewed 2 out of 2 changed files in this pull request and generated no comments.
| File | Description |
|---|---|
src/core/echarts.ts |
Safely initializes optional export options. |
test/ut/spec/api/getConnectedDataURL.test.ts |
Covers supported invocation forms. |
💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
plainheart
left a comment
There was a problem hiding this comment.
Hi, just left some comments. Can you change the target to the release branch so that I include it to v6.1.1?
There was a problem hiding this comment.
Since we have ensured opts, I suggest removing the redundant opts && guard.
d196f40 to
52b4f2b
Compare
|
Removed the three redundant |
Brief Information
This pull request is in the type of:
What does this PR do?
Makes the
optsargument ofechartsInstance.getConnectedDataURLactually optional, as the rest of the method already assumes.Fixed issues
Details
Before: What was the problem?
Calling the documented public API with no argument always threw:
optsis declared optional and every other read of it inside the method is writtendefensively, but the very first read is not:
So the method is internally inconsistent: the
opts &&guards further down and thedelegation to
getDataURL(which normalizesoptsitself) are unreachable for ano-argument call, because line 1026 has already thrown. The failure does not depend
on whether the chart is connected, on the renderer, or on the environment.
After: How does it behave after the fixing?
optsis normalized once at the top, mirroringgetDataURL:getConnectedDataURL(),getConnectedDataURL(undefined)andgetConnectedDataURL({...})all behave as documented. The existingopts && ...reads are left alone — they are now simply redundant rather than load-bearing.
Document Info
One of the following should be checked.
Misc
Security Checking
ZRender Changes
Related test cases or examples to use the new APIs
Added
test/ut/spec/api/getConnectedDataURL.test.ts: no argument, explicitundefined, and an explicit options object. The first two fail onmaster; thethird passes before and after and is kept as a control.
npm run test,npx tsc --noEmitandeslinton the changed file all pass.Merging options
Other information
This is a different problem from #19278, which is about the SVG branch of the same
method; this PR does not address that issue.